Use `transaction-payment`'s congestion modifier for Ethereum base-fee #1765

notlesh · 2022-08-19T16:31:13Z

What does it do?

This PR makes several significant fee modifications:

It replaces the fixed Ethereum fee multiplier with a dynamic one. Currently we ignore the max_fee_per_gas and replace it with a fixed 1 Gwei.
Unifies the multiplier for both Substrate and Ethereum transactions by deriving both of them from pallet transaction-payment's multiplier. While the multiplier has a slightly different impact on the two fee mechanisms, the trend is very similar.
- Note that this fee mechanism has a similar response to EIP-1559 in that it goes up during congestion and down during light blocks.
Tweaks several parameters of the transaction-payment multiplier algorithm so that it should (1) remain within a useful range and (2) update quickly (tuned roughly to double within one hour when blocks are completely full).
- In particular, we bound the fee mechanism to min 0.1 and max 100_000.

⚠️ Breaking Changes ⚠️

All changes are moonbase only
Fixed 1 Gwei gas price (base_fee) removed in favor of pallet-transaction-payment's congestion-based multiplier (acts similarly to EIP-1559)
Min and max multiplier enforced (in terms of base-fee, this is 125_000_000 and 125_000_000_000_000 respectively)
The same multiplier is used for Substrate-based transactions. While the multiplier scales differently with the two txn types, the trend should be similar.

Notes on compatibility with EIP-1559:

This causes us to intentionally deviate from EIP-1559. While our fee mechanism was never compatible and wasn't expected to be strictly compatible, this is a conscious deviation from its entire fee update mechanism.
Our transaction-fee calculation is now compatible with EIP-1559. Specifically, given a known base-fee and a txn max_fee_per_gas and max_priority_fee_per_gas, we will deduct the same fee as Ethereum will under EIP-1559.
The destination of these fees still differs (and is unchanged in this PR). Under EIP-1559, base_fee is burned and any applicable priority_fee is paid to miners. In Moonbeam we still burn 80% and pay 20% to the treasury for both fees. No part of fees are paid to block authors currently.
The adjustment of base_fee is very different than EIP-1559. The specifics of the algorithm can be found in pallet transaction-payment's documentation.

Notes on how this multiplier impacts Substrate and Ethereum fees differently:

Ethereum fees are more heavily impacted by the multiplier than Substrate ones. The effect of changes to the multiplier will be amplified for Ethereum transactions. This is because the multiplier applies to the full Ethereum gas_used whereas it applies only to the weight portion of Substrate fees.
Ethereum fees scale linearly with no base. The multiplier has a lower bound above 0, but if it were to somehow be set to 0, Ethereum would exhibit 0-priced fees (gas_used * 0, basically).
Substrate has several fee mechanisms that are unaffected by the multiplier. So some transactions will seem to be impacted more than others. Specifically:
- base_fee that converts to a flat currency fee
- length_fee
- tip
- (read more here)

Analysis

The fees converge as the network becomes more congested (logarithmic scale). If the network remains empty, then the fees are also held at a constant value.

The following table exhibits the effect of the multiplier starting from 0.000003 (currently in moonriver) to increasing magnitudes. We see that the substrate and evm fees converge on higher multiplier values but substrate fees will always be higher than evm fees by a factor of base + length fees.

Multiplier                      Fees
------------------------------------
  0.000003        12,746,162,435,424 | substrate
                         147,518,810 | evm
                  12,746,014,916,614 | diff
                              199.9% | diff%
                         147,548,488 | substrate-weight-fee-part
                  12,500,000,000,000 | substrate-base-fee-part
------------------------------------ | 
     0.001        12,783,532,386,936 | substrate
                      37,517,500,000 | evm
                  12,746,014,886,936 | diff
                              198.8% | diff%
                      37,517,500,000 | substrate-weight-fee-part
                  12,500,000,000,000 | substrate-base-fee-part
------------------------------------ | 
       0.1        16,497,764,886,936 | substrate
                   3,751,750,000,000 | evm
                  12,746,014,886,936 | diff
                              125.8% | diff%
                   3,751,750,000,000 | substrate-weight-fee-part
                  12,500,000,000,000 | substrate-base-fee-part
------------------------------------ | 
         1        50,226,016,145,686 | substrate
                  37,480,001,258,750 | evm
                  12,746,014,886,936 | diff
                                 29% | diff%
                  37,480,001,258,750 | substrate-weight-fee-part
                  12,500,000,000,000 | substrate-base-fee-part
------------------------------------ | 
        10       387,546,027,474,436 | substrate
                 374,800,012,587,500 | evm
                  12,746,014,886,936 | diff
                                3.3% | diff%
                 374,800,012,587,500 | substrate-weight-fee-part
                  12,500,000,000,000 | substrate-base-fee-part
------------------------------------ | 
       100     3,760,746,140,761,936 | substrate
               3,748,000,125,875,000 | evm
                  12,746,014,886,936 | diff
                                0.3% | diff%
               3,748,000,125,875,000 | substrate-weight-fee-part
                  12,500,000,000,000 | substrate-base-fee-part

From the above table the range of 1.0 - 200.0 seems good so far.

TODO

Find solution for no GenesisConfig in transaction-payment
Fix existing test cases
Tune values to work for various transaction types
New tests
Document breaking changes

…s base-fee

runtime/moonbase/src/lib.rs

boundless-forest · 2022-09-08T08:47:40Z

The NextFeeMultiplier is much similar to the base-fee pallet but on the substrate side. Why not use base-fee pallet here?

nbaztec · 2022-09-09T13:26:14Z

The NextFeeMultiplier is much similar to the base-fee pallet but on the substrate side. Why not use base-fee pallet here?

@AsceticBear The initial reason being having to maintain/re-implement less code/functionality which is quite easy to get wrong (e.g. fees will never recover or continuously drift in a single direction).
The original motivation we have here is to adapt the EVM fees to the changing network traffic and substrate already provides us with a well-tested mechanism in place - this also means the network fee would be more in tandem between Substrate and EVM.

notlesh · 2022-09-13T21:02:28Z

The NextFeeMultiplier is much similar to the base-fee pallet but on the substrate side. Why not use base-fee pallet here?

A bit more detail:

I explored using this and came to the conclusion that EIP1559's design isn't well suited for a network that doesn't experience consistent congestion.

Basically, I think the original design creates a feedback mechanism that keeps the congestion modifier within a reasonable range, something like this:

congestion up -> fees up -> fewer txns eligible -> blocks less full -> congestion down

and

congestion down -> fees down -> more txns eligible -> blocks more full -> congestion up

Without this, it becomes critical to tune pallet base-fee's Threshold parameters because incorrect tuning will create a permanent upward or downward trend. The pallet has no bounds on the fee itself (nor does EIP1559) which can eventually result in fees being either near-zero or impossibly expensive.

While pallet transaction-payment has a similar mechanism, it has far more knobs to tune (including a minimum fee) to alleviate this.

runtime/moonbase/src/lib.rs

librelois · 2023-01-09T15:34:13Z

runtime/moonbase/src/lib.rs

-	/// This value is currently only used by pallet-transaction-payment as an assertion that the
-	/// next multiplier is always > min value.
-	pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000_000u128);
+	pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000);
 	/// Maximum multiplier. We pick a value that is expensive but not impossibly so; it should act
 	/// as a safety net.
 	pub MaximumMultiplier: Multiplier = Multiplier::from(100_000u128);


Shouldn't we add a migration to initialize the FeeMultiplier amount? Today it seems to be worth 1000000000000 on moonbase, moonriver in moonbeam, which is not in the expected range.

Good question. I've explored this pretty thoroughly, here's my summary:

For new blockchains, the genesis value will kick in (this PR made that possible)

For existing blockchains, the minimum and maximum multipliers in transaction-payment will kick in immediately at the end of the runtime upgrade block

A migration could force the multiplier bounds to be enforced earlier (before extrinsics are charged fees for the runtime upgrade block), but this doesn't seem like a big deal to me.

To elaborate:

In transaction-payment's on_finalize(), the TargetedFeeAdjustment::convert() will check the new min and max bounds and enforce them. This value will then be updated in pallet storage and will cause the next block to have fees within bounds.

girazoki

Small questions but overall looks good

girazoki · 2023-01-17T11:51:23Z

tests/tests/test-fees/test-length-fees.ts

@@ -36,7 +36,7 @@ describeDevMoonbeam(
  (context) => {
    it("should have low balance transfer fees", async () => {
      const fee = await testBalanceTransfer(context);
-      expect(fee).to.equal(20958001520875n);
+      expect(fee).to.equal(79_359_001_520_875n);


Is there a way to fetch this through a runtime API? I feel hardcoding this value is prone to having to change it every time

It would be nice to avoid hard coding fee values everywhere, they break nearly all the time.

We have verifyBlockFees, which could be suitable here. Note that it calculates the fees by hand, as opposed to asking the node/runtime to verify it, which I think is important for some of these tests.

Maybe we need an equivalent for a single extrinsic...

I thought there was a payment info runtime API that could return the expected fee, is that not suitable for this?

tests/tests/test-precompile/test-precompile-xtokens.ts

girazoki · 2023-01-17T11:53:06Z

tests/tests/test-precompile/test-precompile-xtokens.ts

@@ -158,7 +159,7 @@ describeDevMoonbeamAllEthTxTypes("Precompiles - xtokens", (context) => {
  });
 });

-describeDevMoonbeamAllEthTxTypes("Precompiles - xtokens", (context) => {
+describeDevMoonbeam("Precompiles - xtokens", (context) => {


Is this intentional because it takes EIP-1559 by default?

It actually takes legacy by default: https://github.com/PureStake/moonbeam/blob/24fddc3701e3e40c2f8febcc4ba1f0af7a49e144/tests/util/setup-dev-tests.ts#L81

I found this a bit surprising, but maybe it make a lot of sense to do it that way when we first introduced EIP1559.

Anyway, IIRC, the reason for this change is because the test uses the TRANSACTION_TEMPLATE which uses gasPrice instead of maxFeePerGas/maxPriorityFeePerGas. I'll rerun the test and see if I can figure out why exactly this was a problem.

Yes, so the problem is the way createTransaction handles maxPriorityFeePerGas, which I think could be improved (I'll open an internal issue to track that).

Basically, in the case of (1) EIP-1559 txn with (2) gasPrice provided but no maxFeePerGas, it will specify maxPriorityFeePerGas: 0, which leads to a discrepancy in the exact fees charged between txn types:

export const createTransaction = async ( context: DevTestContext, options: TransactionOptions ): Promise<string> => { const isLegacy = context.ethTransactionType === "Legacy"; const isEip2930 = context.ethTransactionType === "EIP2930"; const isEip1559 = context.ethTransactionType === "EIP1559"; const gasPrice = options.gasPrice !== undefined ? options.gasPrice : DEFAULT_TXN_MAX_BASE_FEE; const maxPriorityFeePerGas = options.maxPriorityFeePerGas !== undefined ? options.maxPriorityFeePerGas : 0;

tgmichel · 2023-02-03T11:06:04Z

runtime/moonbase/src/lib.rs

+		// is computed in frontier, but that's currently unavoidable.
+		let min_gas_price = TransactionPayment::next_fee_multiplier()
+			.saturating_mul_int(currency::WEIGHT_FEE.saturating_mul(WEIGHT_PER_GAS as u128));
+		(min_gas_price.into(), Weight::zero())


I noticed that we read TransactionPayment::next_fee_multiplier() but we still account zero weight for the min_gas_price call. Is next_fee_multiplier expected to be cached and the weight in the min_gas_price call context to be negligible? Or what is the reason?

Yes, this should be more than 0, will PR a fix soon :)

@notlesh Think I vaguely recall us discussing this and you mentioned that since this is read way too often, we could offer this for free (since it's cached more or less). Not sure if that condition still applied.

…moonbeam-foundation#1765) * Convert transaction-payments congestion modifier to replace Ethereum's base-fee * Remove outdated comment * fix precision bug * import * use saturating mul, improve doc * update fee parameters, add tests * make tests similar * add tests to all runtimes * fix tests * Add transaction-payment GenesisConfig to initialize its Multiplier * Fix some moonbase tests * Rename FixedGasPrice -> TransactionPaymentAsGasPrice * fmt * More ts test fixes * Fixes staking locks * Fix existential balance tests * Fix length fee tests * Query base_fee in createTransaction when needed * Fixes gasPrice for existential test * Fix deposit/fee check for multiple deposit * Fix merge issue * Use legacy txns for some tests that expect full gas_price to be charged * Bump gas price for tests * Update tests to reflect createTransfer default * Use constant for DEFAULT_TXN_MAX_BASE_FEE * Reflect txn values to reflect base_fee change * Prefer constant over literal value * Overhaul fee calculations in verifyBlockFees * Bound effectiveTipPerGas to 0 * Fix substrate-based fees * Overestimate delegation count weight hint * Use legacy txns and expect full gas_price to be paid as fee * Use constant for gas limit value * Start test case for max possible fee conditions * Clean up * Add runtime-upgrade test * prettier * First look at max possible fee * fix auto-compound delegation tx order flakiness * prettier * Remove cargo override (oops) * Hack to fix race condition * prettier * Use beforeEach for setting max multiplier * Test multiplier against Fibonacci contract * prettier * prettier * fmt * Various minor fixes * Add some fee multiplier scenarios * Don't use EXTRINSIC_BASE_WEIGHT in gas calc * Don't expect genesis value at each beforeAll * Bump expectation of CREATE cost * Slightly change length fee assumptions * Use higher gas price * Remove base_fee test * fmt * toml sort * Resolve compiler warnings * More compiler warning fixes * Fix receipt/status test * Remove irrelevant tests * Use maxDelegationCount for weight hint * Remove comment * Remove ignored tests * Move fee tests to integration_tests.rs * Re-remove ignored test * Move moonbeam runtime fee tests to integration_tests * Move moonriver runtime fee tests to integration_tests * fmt * Use base fee constant for gas price * Revert test name change... Co-authored-by: Nisheeth Barthwal <[email protected]> Co-authored-by: Crystalin <[email protected]> Co-authored-by: tgmichel <[email protected]>

notlesh and others added 4 commits August 19, 2022 10:00

Convert transaction-payments congestion modifier to replace Ethereum'…

a0fc51b

…s base-fee

Remove outdated comment

52bf3f8

fix precision bug

a182d56

import

2bdfb9e

nbaztec added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Sep 2, 2022

notlesh commented Sep 2, 2022

View reviewed changes

runtime/moonbase/src/lib.rs Outdated Show resolved Hide resolved

notlesh commented Sep 2, 2022

View reviewed changes

runtime/moonbase/src/lib.rs Outdated Show resolved Hide resolved

use saturating mul, improve doc

b3e84c4

boundless-forest mentioned this pull request Sep 8, 2022

Align evm min_gas_price with substrate darwinia-network/darwinia#963

Closed

nbaztec added 3 commits September 9, 2022 14:07

update fee parameters, add tests

3af5c6c

make tests similar

94e6e0e

add tests to all runtimes

a83778d

fix tests

cac3a2e

notlesh mentioned this pull request Sep 13, 2022

Unify fee modifiers #1420

Closed

3 tasks

notlesh added 2 commits October 6, 2022 21:44

Merge branch 'master' into notlesh-use-transaction-payment-for-base-fee

167d566

Add transaction-payment GenesisConfig to initialize its Multiplier

4b58131

librelois mentioned this pull request Oct 10, 2022

runtime-1900 release #1858

Closed

24 tasks

girazoki reviewed Oct 10, 2022

View reviewed changes

runtime/moonbase/src/lib.rs Outdated Show resolved Hide resolved

girazoki reviewed Oct 10, 2022

View reviewed changes

runtime/moonbase/src/lib.rs Outdated Show resolved Hide resolved

girazoki reviewed Oct 10, 2022

View reviewed changes

runtime/moonbase/src/lib.rs Outdated Show resolved Hide resolved

notlesh and others added 6 commits October 10, 2022 11:14

Merge branch 'master' into notlesh-use-transaction-payment-for-base-fee

66a1d9b

Fix some moonbase tests

86e6d41

Rename FixedGasPrice -> TransactionPaymentAsGasPrice

e55c392

fmt

ccce143

More ts test fixes

751bede

Merge branch 'master' into notlesh-use-transaction-payment-for-base-fee

55058d5

tgmichel and others added 5 commits December 14, 2022 16:47

Fix receipt/status test

fc0ab3d

Remove irrelevant tests

86978cf

Use maxDelegationCount for weight hint

84b9f5d

Remove comment

0c3866e

Remove ignored tests

d56ea3f

notlesh added the breaking Needs to be mentioned in breaking changes label Jan 8, 2023

notlesh added 7 commits January 8, 2023 12:12

Move fee tests to integration_tests.rs

14f9b7a

Merge branch 'master' into notlesh-use-transaction-payment-for-base-fee

5988fb2

Re-remove ignored test

0890e92

Move moonbeam runtime fee tests to integration_tests

160f82f

Move moonriver runtime fee tests to integration_tests

23c9942

fmt

d84a467

Use base fee constant for gas price

16fa4e8

librelois reviewed Jan 9, 2023

View reviewed changes

notlesh requested a review from nbaztec January 10, 2023 16:07

girazoki approved these changes Jan 17, 2023

View reviewed changes

librelois mentioned this pull request Jan 17, 2023

runtime-2100 release #2045

Closed

19 tasks

Revert test name change...

dd5b4cf

librelois approved these changes Jan 17, 2023

View reviewed changes

notlesh merged commit f90be04 into master Jan 17, 2023

notlesh deleted the notlesh-use-transaction-payment-for-base-fee branch January 17, 2023 16:58

notlesh mentioned this pull request Jan 17, 2023

Fix para para-tests (bump gas price) #2047

Merged

timbrinded mentioned this pull request Jan 30, 2023

Added smoke tests for dynamic fee monitoring #2069

Merged

notlesh mentioned this pull request Feb 1, 2023

Clean up min multiplier #2070

Merged

1 task

tgmichel reviewed Feb 3, 2023

View reviewed changes

notlesh mentioned this pull request Feb 7, 2023

Add Dynamic Fee support for Moonriver #2078

Merged

5 tasks

notlesh mentioned this pull request Mar 16, 2023

Add Dynamic Fee support for Moonbeam #2166

Merged

5 tasks

notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Apr 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `transaction-payment`'s congestion modifier for Ethereum base-fee #1765

Use `transaction-payment`'s congestion modifier for Ethereum base-fee #1765

notlesh commented Aug 19, 2022 •

edited

Loading

boundless-forest commented Sep 8, 2022

nbaztec commented Sep 9, 2022

notlesh commented Sep 13, 2022

librelois Jan 9, 2023

notlesh Jan 9, 2023

notlesh Jan 9, 2023

girazoki left a comment

girazoki Jan 17, 2023

notlesh Jan 17, 2023

girazoki Jan 17, 2023

girazoki Jan 17, 2023

notlesh Jan 17, 2023

notlesh Jan 17, 2023

tgmichel Feb 3, 2023

notlesh Feb 3, 2023

nbaztec Feb 3, 2023

Use transaction-payment's congestion modifier for Ethereum base-fee #1765

Use transaction-payment's congestion modifier for Ethereum base-fee #1765

Conversation

notlesh commented Aug 19, 2022 • edited Loading

What does it do?

⚠️ Breaking Changes ⚠️

Analysis

TODO

boundless-forest commented Sep 8, 2022

nbaztec commented Sep 9, 2022

notlesh commented Sep 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

girazoki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Use `transaction-payment`'s congestion modifier for Ethereum base-fee #1765

Use `transaction-payment`'s congestion modifier for Ethereum base-fee #1765

notlesh commented Aug 19, 2022 •

edited

Loading